docs: add parsing request by jannyHou · Pull Request #1519 · loopbackio/loopback-next

jannyHou · 2018-07-11T21:33:44Z

Connect to #1489

The story only contains validation doc, while I realized the doc for action parsing request is missing, so I added it and also the coercion doc.

Checklist

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

bajtos

Nice 👍

I think there may be few places with incorrect grammar, I'll let somebody more fluent in English to review the language.

shimks

Overall content looks fab 🤩
🧤It's time for a grammar shake down, I hope you're ready

shimks · 2018-07-12T18:04:33Z

+rest decorators.
+
+```ts
+class TodoController() {


nitpick: this.todoRepo is not specified in this TS class

No parenthesis

shimks · 2018-07-12T18:04:51Z

+```
+
+An operation specification will be generated in-memory to describe it, and raw
+datas are parsed from request according to the specification. For example the


data does not have a plural form

Did you mean For example, if the first parameter ...?

I meant the example code above, rephrased it to:

In the example
above, the first parameter is from source path, then its value will be parsed
from a request's path.

shimks · 2018-07-12T18:08:32Z

+
+### Coercion
+
+The parameters parsed from path, header, query of a http request are always in


path, header and query (American way of using and)

It also might be good to provide context that it's node.js's request parameters being in string format

shimks · 2018-07-12T18:12:02Z

+need to be converted to its corresponding JavaScript runtime type, which is
+inferred from its parameter specification.
+
+Give the example operation `replaceTodo` in section


Take instead of Give might be a better choice of word

shimks · 2018-07-12T18:12:07Z

+inferred from its parameter specification.
+
+Give the example operation `replaceTodo` in section
+[parsing raw data](#parsing-raw-data), which takes in a number `id` as the first


no comma here

combined with the comment above, do you mean rephrase it to:

Take the example operation replaceTodo in section
parsing raw data which takes in a number id as the first
input, coercion means users don't need to do a type conversion by themselves in
the controller function like below:

?

Nevermind, please put the comma back in. 🙇

shimks · 2018-07-12T18:29:07Z

+
+_See [model](Model.md) to know more details about how to decorate a model class_
+
+When the `PUT` method `/todo/{id}` get called, the `todo` instance from request


the request body

shimks · 2018-07-12T18:30:07Z

+When the `PUT` method `/todo/{id}` get called, the `todo` instance from request
+body will be validated with a well defined specification.
+
+Section [RequestBody Decorator](Decorators.md#requestbody-decorator) has a very


Section on requestBody Decorator

shimks · 2018-07-12T18:30:28Z

+
+Section [RequestBody Decorator](Decorators.md#requestbody-decorator) has a very
+detailed explanation of the various ways to provide a request body
+specification. It would be a redundant to repeat it here. So make sure you read


I don't think we need the last two sentences

hmm I do want to emphasize "make sure you read that section", I deleted the fist sentence.

How about this:
Validation of model objects is heavily dependent on its OpenAPI Schema defined in/by @requestBody decorator. Please refer to the documentation on [@requestBody decorator](link) to get a comprehensive idea of defining custom validation rules for your models.

cool 😎 !

shimks · 2018-07-12T18:49:47Z

+
+#### Localizing errors
+
+A body data may contain multiple invalid things, like missing required field,


A body data could break multiple validation rules, ... things is I think too informal here.

shimks · 2018-07-12T18:51:40Z

@@ -229,7 +229,7 @@ How to get query string param values.

 ### Parsing Requests


If we're going to have a subpage for this section, we should get rid of its mention entirely here.
Maybe we can also consider adding a link

important catch! thx

shimks · 2018-07-12T20:07:57Z

+
+An operation specification will be generated in-memory to describe it, and raw
+data are parsed from request according to the specification. In the example
+above, the first parameter is from source `path`, then its value will be parsed


path, so its value will be parsed from a request's path

shimks · 2018-07-12T20:10:10Z

+inferred from its parameter specification.
+
+Give the example operation `replaceTodo` in section
+[parsing raw data](#parsing-raw-data), which takes in a number `id` as the first


Nevermind, please put the comma back in. 🙇

shimks

LGTM.
There's a couple of outlying comments I've made that were collapsed, so please check them out as well~~

b-admike · 2018-07-12T20:12:37Z

+}
+```
+
+An operation specification will be generated in-memory to describe it, and raw


nitpick: An OpenAPI operation specification

b-admike · 2018-07-12T20:12:56Z

+```
+
+An operation specification will be generated in-memory to describe it, and raw
+data are parsed from request according to the specification. In the example


another nitpick: and raw data are parsed from request -> and raw data is parsed from the request

b-admike · 2018-07-12T20:33:46Z

+
+You can specify a parameter's type by calling shortcut decorators of `@param`
+like `@param.query.integer()`. A list of available shortcuts can be found in the
+[api documents](https://apidocs.strongloop.com/@loopback%2fdocs/openapi-v3.html#param).


nitpick: api documents -> API Docs

b-admike · 2018-07-12T20:34:25Z

+
+Here are our default validation rules for each type:
+
+- number: validated by `isNaN(Number(data))`.


nitpick: I suggest to either remove . for each line of the list or add it to be consistent

@bajtos hmm...I added it for each line....which line did you find not inconsistent?

b-admike · 2018-07-12T20:35:08Z

+#### Request Body
+
+The data from request body is validated against its OpenAPI schema
+specification. We use module [AJV](https://github.com/epoberezkin/ajv) to


nitpick: module AJV -> AJV module

b-admike · 2018-07-12T20:37:50Z

+```ts
+import {Todo} from './models';
+
+...


minor nitpick: can we wrap this code block with the controller class declaration?

import {Todo} from './models'; class test { ... @put('/todos/{id}') async replaceTodo( @param.path.number('id') id: number, @requestBody() todo: Todo, ): Promise<boolean> { return await this.todoRepo.replaceById(id, todo); } ... }

it just didn't render properly for me in VS code :P

There are too many irrelevant code in the class definition which distract users when reading :( that's why I deleted them.

it just didn't render properly for me in VS code

Mine looks good 🤔 :

probably has to do with the ...
Commenting those out should do it

b-admike · 2018-07-12T20:38:56Z

+
+{% include note.html title="Model documentation" content="See [model](Model.md) to know more details about how to decorate a model class" %}
+
+When the `PUT` method `/todo/{id}` gets called, the `todo` instance from the


nitpick: 'PUT' method on path '/todo/{id}'

b-admike · 2018-07-12T20:40:18Z

+- If a model property's type refers to another model, make sure it is also
+  decorated with `@model` decorator.
+
+- By the API first approach, you can also provide the request body specification


nitpick: By the API first approach -> If you're using API first development approach

b-admike

LGTM overall, just minor nitpicks; great work!

shimks · 2018-07-13T19:23:24Z

-before it can be used as seen below:
+For example, the operation `replaceTodo` in section
+[parsing raw data](#parsing-raw-data) takes in a number `id` as the first input,
+without coercion,`id` would have to be manually cast into the number type before


as the first input. Without coercion, id would have to ...

shimks · 2018-07-13T19:23:54Z

+```ts
+import {Todo} from './models';
+
+...


probably has to do with the ...
Commenting those out should do it

b-admike

👏

jannyHou requested review from bajtos and raymondfeng as code owners July 11, 2018 21:33

jannyHou force-pushed the doc/validation branch from ff88037 to 001831b Compare July 11, 2018 21:47

jannyHou self-assigned this Jul 11, 2018

bajtos approved these changes Jul 12, 2018

View reviewed changes

shimks reviewed Jul 12, 2018

View reviewed changes

jannyHou force-pushed the doc/validation branch from c8ec162 to 5e585dc Compare July 12, 2018 19:58

shimks reviewed Jul 12, 2018

View reviewed changes

shimks approved these changes Jul 12, 2018

View reviewed changes

b-admike reviewed Jul 12, 2018

View reviewed changes

b-admike approved these changes Jul 12, 2018

View reviewed changes

jannyHou force-pushed the doc/validation branch from 5e585dc to ec14a04 Compare July 13, 2018 19:21

shimks approved these changes Jul 13, 2018

View reviewed changes

b-admike approved these changes Jul 13, 2018

View reviewed changes

bajtos approved these changes Jul 16, 2018

View reviewed changes

jannyHou force-pushed the doc/validation branch from ec14a04 to 8066c61 Compare July 16, 2018 14:39

virkt25 mentioned this pull request Jul 16, 2018

Property 'body' does not exist on type 'ParsedResponse' #1362

Closed

jannyHou force-pushed the doc/validation branch from 8066c61 to 99839c6 Compare July 16, 2018 15:40

docs: add parsing request

f6e3eee

jannyHou force-pushed the doc/validation branch from 99839c6 to f6e3eee Compare July 16, 2018 16:02

jannyHou merged commit 6560f25 into master Jul 16, 2018

jannyHou deleted the doc/validation branch July 16, 2018 20:09


		### Coercion

		The parameters parsed from path, header, query of a http request are always in


		_See [model](Model.md) to know more details about how to decorate a model class_

		When the `PUT` method `/todo/{id}` get called, the `todo` instance from request


		#### Localizing errors

		A body data may contain multiple invalid things, like missing required field,

		@@ -229,7 +229,7 @@ How to get query string param values.

		### Parsing Requests


		Here are our default validation rules for each type:

		- number: validated by `isNaN(Number(data))`.


		{% include note.html title="Model documentation" content="See [model](Model.md) to know more details about how to decorate a model class" %}

		When the `PUT` method `/todo/{id}` gets called, the `todo` instance from the

Uh oh!

Conversation

jannyHou commented Jul 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

bajtos left a comment

Choose a reason for hiding this comment

Uh oh!

shimks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shimks Jul 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shimks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jannyHou Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jannyHou commented Jul 11, 2018 •

edited

Loading

shimks Jul 12, 2018 •

edited

Loading

jannyHou Jul 13, 2018 •

edited

Loading